[bugfix] fix tie_word_embeddings#150
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the preservation logic for untie_embeddings_and_output_weights in src/mcore_bridge/config/parser.py. The reviewer points out that removing this logic entirely can cause issues when hf_config.name_or_path is not set, as sub-config defaults might overwrite parent config settings. They suggest a more refined preservation logic that only restores the setting if it was explicitly set to True in the parent config.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for key in ['text_config', 'llm_config', 'thinker_config']: | ||
| if hasattr(config, key): | ||
| megatron_config.update(_convert_config(getattr(config, key), _internal_call=True)) |
There was a problem hiding this comment.
Removing the preservation of untie_embeddings_and_output_weights entirely can lead to issues when hf_config.name_or_path is not set (e.g., during programmatic initialization or testing). In such cases, the sub-config's default tie_word_embeddings = True (which maps to untie_embeddings_and_output_weights = False) will overwrite the parent config's explicit or class-default tie_word_embeddings = False (which maps to untie_embeddings_and_output_weights = True).
Instead of removing the preservation logic completely, we can make it smarter: only restore untie_embeddings_and_output_weights if the parent config had it set to True (untied). This prevents the sub-config's default False (tied) from overwriting the parent's explicit True (untied), while still allowing a sub-config's explicit True (untied) to override the parent's default False (tied).
untie_embeddings_and_output_weights = megatron_config.get('untie_embeddings_and_output_weights')
for key in ['text_config', 'llm_config', 'thinker_config']:
if hasattr(config, key):
megatron_config.update(_convert_config(getattr(config, key), _internal_call=True))
if untie_embeddings_and_output_weights is True:
megatron_config['untie_embeddings_and_output_weights'] = untie_embeddings_and_output_weights
1 participant
No description provided.